-
Notifications
You must be signed in to change notification settings - Fork 8k
Add Qbv support, Qbv is Enhancements for Scheduled Traffic (EST) #96124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add Qbv support, Qbv is Enhancements for Scheduled Traffic (EST) #96124
Conversation
subsys/net/lib/shell/qbv_shell.c
Outdated
if (ret < 0) { | ||
return ret; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each command, we should probably verify that the interface is Ethernet type.
Why force user to use the device as a parameter if we are using interface internally? It should make things clearer if user just uses network interface index to configure Qbv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not resolve the configuration but let the reviewer do it. See https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers for details.
We should add here the interface type check, see my comment below.
8bc2f6c
to
f440963
Compare
af4702c
to
95da213
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments for DSA part.
Thanks.
95da213
to
cb451f7
Compare
cb451f7
to
f1b6ba5
Compare
is this for IEEE 802.1Qbv Ethernet? Can you please change the title and body with more detais? |
f1b6ba5
to
fb85304
Compare
d1e9dd6
to
3bafe36
Compare
config NET_SHELL_QBV_SUPPORTED | ||
bool "Qbv Shell" | ||
depends on NET_L2_ETHERNET_MGMT | ||
help | ||
Enable Qbv Shell. | ||
The Qbv shell currently supports set/enable operations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not what I meant. The supported option should look similar as the other options around it.
config NET_SHELL_QBV_SUPPORTED
bool "Qbv shell"
default y
depends on NET_SHELL_SHOW_DISABLED_COMMANDS || NET_QBV
subsys/net/lib/shell/qbv_shell.c
Outdated
if (ret < 0) { | ||
return ret; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not resolve the configuration but let the reviewer do it. See https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers for details.
We should add here the interface type check, see my comment below.
PR_WARNING("No such interface in index %d\n", idx); | ||
return -ENOEXEC; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is missing here is verification that the interface is Ethernet type. That can be done like this
if (net_if_l2(iface) != &NET_L2_GET_NAME(ETHERNET)) {
PR_WARNING("Qbv can be set only for Ethernet\n");
return -ENOEXEC;
}
PR_WARNING("No such interface in index %d\n", idx); | ||
return -ENOEXEC; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interface type check should be added here too. Perhaps you could refactor code for the idx, iface and the interface type check into separate checker function and call that for each sub-shell command.
/* qbv set_config <iface_index> <row> <gate_control> <interval> */ | ||
static int cmd_qbv_set_gc(const struct shell *sh, size_t argc, char **argv) | ||
{ | ||
struct net_if *iface; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change each sub-command to work like this:
static int cmd_qbv_xxx(const struct ...., )
{
#if defined(CONFIG_NET_QBV)
#else
PR_INFO("Set %s to enable %s support.\n", "CONFIG_NET_QBV", "Qbv");
#endif
}
This way user is informed what options to set if Qbv is not enabled. And the code is compiled out if CONFIG_NET_SHELL_QBV_SUPPORTED
is turned off (set to n
, it is y
by default).
Supported set/get_config API. Signed-off-by: Qiang Zhao <[email protected]>
Add NET_QBV in Kconfig, Qbv is Enhancements for Scheduled Traffic (EST), one feature of TSN. The PTP clock provides the time reference for Qbv Signed-off-by: Qiang Zhao <[email protected]>
Add DSA Qbv support, add set_config/get_config to set and get Qbv configuration. support enable/disable, set/get times, set/get list length and set/get gate control list. Signed-off-by: Qiang Zhao <[email protected]>
3bafe36
to
b0d5430
Compare
b0d5430
to
10919db
Compare
Added Qbv shell subcommand to net command. Supported enable, set_config, set_time and get_info functions. Signed-off-by: Qiang Zhao <[email protected]>
add Qbv capability for dsa_nxp_imx_netc Signed-off-by: Qiang Zhao <[email protected]>
10919db
to
0686776
Compare
|
#ifdef CONFIG_NET_QBV | ||
memset(&(prv->qbv_config[cfg->port_idx].tgs_config), 0, sizeof(netc_tb_tgs_gcl_t)); | ||
memset(prv->qbv_config[cfg->port_idx].gcList, 0, | ||
sizeof(netc_tgs_gate_entry_t) * CONFIG_DSA_NXP_NETC_GCL_LEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about replace these two memset with the suggested code at: line 476
if (row > CONFIG_DSA_NXP_NETC_GCL_LEN) { | ||
LOG_ERR("The gate control list length exceeds the limit"); | ||
ret = -ENOTSUP; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
break; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix next version
if (row > CONFIG_DSA_NXP_NETC_GCL_LEN) { | ||
LOG_ERR("The gate control list length exceeds the limit"); | ||
ret = -ENOTSUP; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
break; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will fix next version
Add Qbv support, Qbv(IEEE 802.1Qbv) is Enhancements for Scheduled Traffic (EST), one
feature of TSN. The PTP clock provides the time reference for Qbv
In subsys/net/l2/ethernet/Kconfig, add NET_QBV config
In driver, add set_config/get_config to set and get Qbv configuration.
support enable/disable, set/get times, set/get list length and
set/get gate control list.
Add a Qbv shell cmd to set/get qbv